Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Prepare proposer #3043

Closed
wants to merge 33 commits into from

Conversation

paulhauner
Copy link
Member

@paulhauner paulhauner commented Feb 27, 2022

Issue Addressed

Resolves #2936

Proposed Changes

Adds functionality for calling validator/prepare_beacon_proposer in advance.

There is a BeaconChain::prepare_beacon_proposer method which, which called, computes the proposer for the next slot. If that proposer has been registered via the validator/prepare_beacon_proposer API method, then the beacon_chain.execution_layer will be provided the PayloadAttributes for us in all future forkchoiceUpdated calls. An artificial forkchoiceUpdated call will be created 4s before each slot, when the head updates and when a validator updates their information.

Additionally, I added strict ordering for calls from the BeaconChain to the ExecutionLayer. I'm not certain the ExecutionLayer will always maintain this ordering, but it's a good start to have consistency from the BeaconChain. There are some deadlock opportunities introduced, they are documented in the code.

Additional Info

@paulhauner paulhauner added blocked work-in-progress PR is a work-in-progress bellatrix Required to support the Bellatrix Upgrade labels Feb 27, 2022
@paulhauner paulhauner self-assigned this Feb 28, 2022
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a couple comments around simplifying this, but let me know if there are edge cases I am missing that are informing the design here.

Generally I was thinking the easiest/safest way to maintain BN <> EL consistency is to minimize when we call notify_forkchoice_updated and to only call it from fork_choice. We'd be relying on fork choice's ordering which seems like the correct thing to do to me but please correct me here otherwise.

The one place outside of fork choice where calling notify_forkchoice_updated seems unavoidable is when we fallback from get_payloadV1 due to a payload id missing from our cache (here). But calling a synthetic fork choice update here doesn't really seem like a good thing anyways... could we just run a call to chain.fork_choice() in this scenario instead?

If we're able to only call notify_forkchoice_updated from fork choice, I think we could then remove all the notify_forkchoice_updated specific locking that's added here as well.

//
// This seems OK. It's not a significant waste of EL<>CL bandwidth or resources, as
// far as I know.
if let Err(e) = self.prepare_beacon_proposer_blocking() {
Copy link
Member

@realbigsean realbigsean Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of running this after update_execution_engine_forkchoice_blocking? This seems intentional, but I am only really seeing the potential for duplicate notify_forkchoice_updated requests, which like you point out doesn't seem bad but I'm not seeing why it might be useful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So prepare_beacon_proposer_blocking will only send a fcU (forkchciceUpdated) message to the EL if something has changed or if the timing is right.

As it's currently written, we need to call prepare_beacon_proposer_blocking and notify_forkchoice_updated to ensure that at least one fcU call goes through to the EE.

We could probably avoid this by breaking out the fcU call from prepare_beacon_proposer_blocking it get it to return a "should call fcU bool". I'm not certain it's worth it, but I'll give it some thought.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling fcU before calculating the proposer in order to notify the EE as soon as possible seems reason enough to me to keep it as is 👌

}
}

/// Loop indefinitely, calling `BeaconChain::prepare_beacon_proposer_async` at an interval.
Copy link
Member

@realbigsean realbigsean Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this service covers the scenario where we have a skip slot right before we propose, and want to make sure we still notify the execution engine of our PayloadAttributes. But shouldn't that already be covered by the state_advance_timer which triggers at the same time and calls fork_choice which calls prepare_beacon_proposer_async?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thinking, but unfortunately the state_advance_timer only moves forward a single slot:

// Protect against advancing a state more than a single slot.
//
// Advancing more than one slot without storing the intermediate state would corrupt the
// database. Future works might store temporary, intermediate states inside this function.
return Err(Error::BadStateSlot {
_block_slot: head_slot,
_state_slot: state.slot(),
});

So after one skip slot and following slots won't be advanced.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh gotcha, didn't realize this

@@ -2229,6 +2229,13 @@ pub fn serve<T: BeaconChainTypes>(
)
})?;

chain.prepare_beacon_proposer_blocking().map_err(|e| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sort of think this endpoint should only handle updating the proposer preparation cache, and we should make sure fork choice is the only place we call prepare_beacon_proposer_async from.

I could see the benefit of this potentially allowing last second suggested fee recipients changes. But I'm not sure that's something we should try to code for. To me it seems like the later this information is provided the less profitable the block could be and the more likely it is to just be missed completely, so there's already strong incentives to provide the fee recipient early and not to change it frequently.

If we are worried about not being able to get the fee recipient on startup for validators proposing immediately, we could also persist this cache or allow the beacon chain to read from the suggested fee recipients file.

@paulhauner
Copy link
Member Author

paulhauner commented Mar 2, 2022

Generally I was thinking the easiest/safest way to maintain BN <> EL consistency is to minimize when we call notify_forkchoice_updated and to only call it from fork_choice.

Even if we do call it inside fork choice, we still need to deal with ordering. Even if the call to fcU is inside the fork_choice_internal function, it still doesn't mean the calls are ordered. Something can call fork_choice_internal multiple times, unless we've prevented concurrency internally via some lock. We could use the main chain.fork_choice lock, however I'm hesitant to hold that lock whilst we call out to remote servers. That is likely to cause long-lived contention.

I'm open to just returning a bool from prepare_beacon_proposer_blocking and callers can use that value to call fork choice, if they want. However, I think there's still downsides to that. For example, during fork_choice_internal, it seems valuable to me to call fcU as quickly as possible, then compute the proposers, then maybe call it again. Some ELs might avoid syncing non-head-chains until we provide them a fcU message saying it's the head. In those cases, we want the EL to be aware of the head ASAP so they can start syncing it. In the case where the shuffling isn't in the cache, it seems ideal to send 2x fcU messages; one that alerts about the head change and a later one that informs about the proposer.

@realbigsean
Copy link
Member

Even if we do call it inside fork choice, we still need to deal with ordering.

Yes I was misunderstanding this. I was looking for a way around the mutex lock on fcU because it'll be called from a lot of different places, but I guess it is the only way to guarantee order.

@paulhauner
Copy link
Member Author

It turns out there's a bit of an edge-case with the transition block that isn't covered here. I've documented it in #3058.

I think we should merge this PR as-is since it's a significant improvement and has no issues before/after the transition block. We can come back for #3058.

Squashed commit of the following:

commit c03670e
Author: Paul Hauner <[email protected]>
Date:   Fri Mar 4 09:41:11 2022 +1100

    Filter out zero-hashes

commit 6d269dd
Author: Paul Hauner <[email protected]>
Date:   Fri Mar 4 09:09:28 2022 +1100

    Avoid sending fcU with zero hash
@paulhauner
Copy link
Member Author

Alrighty, I've been testing this on merge-devnet-5 with our 1k validators. It's looking good there so I'm happy to merge.

There are two changes since last review:

  1. Add in the changes from Avoid sending a zero-hash head_block_hash to EEs #3057
  2. Add some metrics.

You can see these changes here: paulhauner/lighthouse@76a1d1d...d029c7b

If I can get an approval for these changes then I'll merge :)

@paulhauner
Copy link
Member Author

This is failing CI due to Geth changing some CLI flags. See #3046 (comment)

@paulhauner
Copy link
Member Author

I've also added 4811406 after observing a panic:

thread 'tokio-runtime-worker' panicked at 'Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.'

That commit has approval from @AgeManning.

paulhauner added a commit that referenced this pull request Mar 8, 2022
Squashed commit of the following:

commit 4811406
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 14:39:58 2022 +1100

    Use spawn_blocking for sync fork choice call

commit b0e0829
Merge: d029c7b 4186d11
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:53:54 2022 +1100

    Merge branch 'unstable' into prepare-proposer

commit d029c7b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:40:40 2022 +1100

    Add metrics

commit 70e3b82
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 09:45:47 2022 +1100

    Squash merge of #3057

    Squashed commit of the following:

    commit c03670e
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:41:11 2022 +1100

        Filter out zero-hashes

    commit 6d269dd
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:09:28 2022 +1100

        Avoid sending fcU with zero hash

commit 76a1d1d
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:54 2022 -0500

    Update beacon_node/beacon_chain/src/beacon_chain.rs

commit 4a7e3a7
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:35 2022 -0500

    Update beacon_node/beacon_chain/tests/payload_invalidation.rs

commit c651994
Merge: bb1f7d0 aea43b6
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:05:26 2022 -0500

    Merge branch 'unstable' into prepare-proposer

commit bb1f7d0
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:07:24 2022 +1100

    Update beacon_node/execution_layer/src/lib.rs

    Co-authored-by: realbigsean <[email protected]>

commit 68e4648
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:04:48 2022 +1100

    Prevent running during sync

commit 5ddeae8
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 10:57:28 2022 +1100

    Spawn proposer prepare routine instead of waiting

commit 2306b09
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 19:01:06 2022 +1100

    Fix failing test

commit 8eca77a
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 18:54:26 2022 +1100

    Fix bugs

commit 15d6b71
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 13:03:40 2022 +1100

    Read proposer during preparation

commit b1ac426
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 20:17:34 2022 +1100

    Add more tests

commit 357ff8b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:36 2022 +1100

    Save prev request in exec layer

commit e19fa44
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:17 2022 +1100

    Upgrade error to dbg

commit d2d0099
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 16:29:02 2022 +1100

    Add forkchoice lock

commit 28b94cb
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:30:55 2022 +1100

    Tidy, add comments

commit b24a3ea
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:09:20 2022 +1100

    Don't start update service if there's no EL

commit 3f56ed0
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:08:52 2022 +1100

    Reduce nesting

commit 61c50b7
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:02:42 2022 +1100

    Call prepare_proposer after head change

commit 8fb610c
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 14:44:51 2022 +1100

    Tidy

commit 7e008cb
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:39:29 2022 +1100

    Add prepare call to HTTP api

commit 44d090e
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:36:32 2022 +1100

    Add proposer prep service

commit 8193fae
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:12:45 2022 +1100

    Add fc call to preparation endpoint

commit 2ad4e18
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:56:45 2022 +1100

    Repair after rebase

commit f1e6056
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:41:34 2022 +1100

    Start adding payload prep delay

commit 0f72e34
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:31:33 2022 +1100

    Add proposer prep to beacon chain

commit 10d545f
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 10:42:08 2022 +1100

    Add `proposers` cache to EL
@paulhauner
Copy link
Member Author

paulhauner commented Mar 8, 2022

Sorry for the noise here. As you can see, I found some bugs whilst testing with our validators on merge-devnet-5.

I think this will require additional review, sorry @realbigsean. Unfortunately the diff between the previously-approved changes and the current commit includes a merge of unstable (I had to resolve a conflict, sorry).

We should be good to go after this round of review, this branch is performing well presently 🚀

…epare-proposer

� Conflicts:
�	Cargo.lock
�	beacon_node/execution_layer/Cargo.toml
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New changes look good to me, left a couple nits. Resolved a merge conflict in Cargo.lock and a Cargo.toml as well to try to get CI running

beacon_node/beacon_chain/tests/payload_invalidation.rs Outdated Show resolved Hide resolved
beacon_node/execution_layer/src/metrics.rs Outdated Show resolved Hide resolved
@paulhauner
Copy link
Member Author

Thanks for the review and CI fix @realbigsean 🙏

I'll see how this goes on CI and the testnets and merge if all goes well.

paulhauner added a commit that referenced this pull request Mar 8, 2022
Squashed commit of the following:

commit f0153f1
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 9 09:39:48 2022 +1100

    Apply suggestions from code review

    Co-authored-by: realbigsean <[email protected]>

commit b6b6a34
Author: realbigsean <[email protected]>
Date:   Tue Mar 8 08:56:23 2022 -0500

    Cargo.lock

commit e270c61
Merge: c86ada0 381d0ec
Author: realbigsean <[email protected]>
Date:   Tue Mar 8 08:55:57 2022 -0500

    Merge branch 'unstable' of https://github.com/sigp/lighthouse into prepare-proposer

    � Conflicts:
    �	Cargo.lock
    �	beacon_node/execution_layer/Cargo.toml

commit c86ada0
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 16:58:39 2022 +1100

    Add payload id hit/miss metrics

commit 4811406
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 14:39:58 2022 +1100

    Use spawn_blocking for sync fork choice call

commit b0e0829
Merge: d029c7b 4186d11
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:53:54 2022 +1100

    Merge branch 'unstable' into prepare-proposer

commit d029c7b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:40:40 2022 +1100

    Add metrics

commit 70e3b82
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 09:45:47 2022 +1100

    Squash merge of #3057

    Squashed commit of the following:

    commit c03670e
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:41:11 2022 +1100

        Filter out zero-hashes

    commit 6d269dd
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:09:28 2022 +1100

        Avoid sending fcU with zero hash

commit 76a1d1d
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:54 2022 -0500

    Update beacon_node/beacon_chain/src/beacon_chain.rs

commit 4a7e3a7
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:35 2022 -0500

    Update beacon_node/beacon_chain/tests/payload_invalidation.rs

commit c651994
Merge: bb1f7d0 aea43b6
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:05:26 2022 -0500

    Merge branch 'unstable' into prepare-proposer

commit bb1f7d0
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:07:24 2022 +1100

    Update beacon_node/execution_layer/src/lib.rs

    Co-authored-by: realbigsean <[email protected]>

commit 68e4648
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:04:48 2022 +1100

    Prevent running during sync

commit 5ddeae8
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 10:57:28 2022 +1100

    Spawn proposer prepare routine instead of waiting

commit 2306b09
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 19:01:06 2022 +1100

    Fix failing test

commit 8eca77a
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 18:54:26 2022 +1100

    Fix bugs

commit 15d6b71
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 13:03:40 2022 +1100

    Read proposer during preparation

commit b1ac426
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 20:17:34 2022 +1100

    Add more tests

commit 357ff8b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:36 2022 +1100

    Save prev request in exec layer

commit e19fa44
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:17 2022 +1100

    Upgrade error to dbg

commit d2d0099
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 16:29:02 2022 +1100

    Add forkchoice lock

commit 28b94cb
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:30:55 2022 +1100

    Tidy, add comments

commit b24a3ea
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:09:20 2022 +1100

    Don't start update service if there's no EL

commit 3f56ed0
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:08:52 2022 +1100

    Reduce nesting

commit 61c50b7
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:02:42 2022 +1100

    Call prepare_proposer after head change

commit 8fb610c
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 14:44:51 2022 +1100

    Tidy

commit 7e008cb
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:39:29 2022 +1100

    Add prepare call to HTTP api

commit 44d090e
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:36:32 2022 +1100

    Add proposer prep service

commit 8193fae
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:12:45 2022 +1100

    Add fc call to preparation endpoint

commit 2ad4e18
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:56:45 2022 +1100

    Repair after rebase

commit f1e6056
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:41:34 2022 +1100

    Start adding payload prep delay

commit 0f72e34
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:31:33 2022 +1100

    Add proposer prep to beacon chain

commit 10d545f
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 10:42:08 2022 +1100

    Add `proposers` cache to EL
paulhauner added a commit to paulhauner/lighthouse that referenced this pull request Mar 9, 2022
Squashed commit of the following:

commit f0153f1
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 9 09:39:48 2022 +1100

    Apply suggestions from code review

    Co-authored-by: realbigsean <[email protected]>

commit b6b6a34
Author: realbigsean <[email protected]>
Date:   Tue Mar 8 08:56:23 2022 -0500

    Cargo.lock

commit e270c61
Merge: c86ada0 381d0ec
Author: realbigsean <[email protected]>
Date:   Tue Mar 8 08:55:57 2022 -0500

    Merge branch 'unstable' of https://github.com/sigp/lighthouse into prepare-proposer

    � Conflicts:
    �	Cargo.lock
    �	beacon_node/execution_layer/Cargo.toml

commit c86ada0
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 16:58:39 2022 +1100

    Add payload id hit/miss metrics

commit 4811406
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 14:39:58 2022 +1100

    Use spawn_blocking for sync fork choice call

commit b0e0829
Merge: d029c7b 4186d11
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:53:54 2022 +1100

    Merge branch 'unstable' into prepare-proposer

commit d029c7b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 10:40:40 2022 +1100

    Add metrics

commit 70e3b82
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 8 09:45:47 2022 +1100

    Squash merge of sigp#3057

    Squashed commit of the following:

    commit c03670e
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:41:11 2022 +1100

        Filter out zero-hashes

    commit 6d269dd
    Author: Paul Hauner <[email protected]>
    Date:   Fri Mar 4 09:09:28 2022 +1100

        Avoid sending fcU with zero hash

commit 76a1d1d
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:54 2022 -0500

    Update beacon_node/beacon_chain/src/beacon_chain.rs

commit 4a7e3a7
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:17:35 2022 -0500

    Update beacon_node/beacon_chain/tests/payload_invalidation.rs

commit c651994
Merge: bb1f7d0 aea43b6
Author: realbigsean <[email protected]>
Date:   Thu Mar 3 14:05:26 2022 -0500

    Merge branch 'unstable' into prepare-proposer

commit bb1f7d0
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:07:24 2022 +1100

    Update beacon_node/execution_layer/src/lib.rs

    Co-authored-by: realbigsean <[email protected]>

commit 68e4648
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 11:04:48 2022 +1100

    Prevent running during sync

commit 5ddeae8
Author: Paul Hauner <[email protected]>
Date:   Thu Mar 3 10:57:28 2022 +1100

    Spawn proposer prepare routine instead of waiting

commit 2306b09
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 19:01:06 2022 +1100

    Fix failing test

commit 8eca77a
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 18:54:26 2022 +1100

    Fix bugs

commit 15d6b71
Author: Paul Hauner <[email protected]>
Date:   Wed Mar 2 13:03:40 2022 +1100

    Read proposer during preparation

commit b1ac426
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 20:17:34 2022 +1100

    Add more tests

commit 357ff8b
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:36 2022 +1100

    Save prev request in exec layer

commit e19fa44
Author: Paul Hauner <[email protected]>
Date:   Tue Mar 1 16:41:17 2022 +1100

    Upgrade error to dbg

commit d2d0099
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 16:29:02 2022 +1100

    Add forkchoice lock

commit 28b94cb
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:30:55 2022 +1100

    Tidy, add comments

commit b24a3ea
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:09:20 2022 +1100

    Don't start update service if there's no EL

commit 3f56ed0
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:08:52 2022 +1100

    Reduce nesting

commit 61c50b7
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 15:02:42 2022 +1100

    Call prepare_proposer after head change

commit 8fb610c
Author: Paul Hauner <[email protected]>
Date:   Mon Feb 28 14:44:51 2022 +1100

    Tidy

commit 7e008cb
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:39:29 2022 +1100

    Add prepare call to HTTP api

commit 44d090e
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:36:32 2022 +1100

    Add proposer prep service

commit 8193fae
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 12:12:45 2022 +1100

    Add fc call to preparation endpoint

commit 2ad4e18
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:56:45 2022 +1100

    Repair after rebase

commit f1e6056
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:41:34 2022 +1100

    Start adding payload prep delay

commit 0f72e34
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 11:31:33 2022 +1100

    Add proposer prep to beacon chain

commit 10d545f
Author: Paul Hauner <[email protected]>
Date:   Sun Feb 27 10:42:08 2022 +1100

    Add `proposers` cache to EL
@paulhauner
Copy link
Member Author

Looks like windows tests are just being dodgy again :(

Everything is looking good on the testnet nodes 🎉

bors r+

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Mar 9, 2022
bors bot pushed a commit that referenced this pull request Mar 9, 2022
## Issue Addressed

Resolves #2936

## Proposed Changes

Adds functionality for calling [`validator/prepare_beacon_proposer`](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/prepareBeaconProposer) in advance.

There is a `BeaconChain::prepare_beacon_proposer` method which, which called, computes the proposer for the next slot. If that proposer has been registered via the `validator/prepare_beacon_proposer` API method, then the `beacon_chain.execution_layer` will be provided the `PayloadAttributes` for us in all future forkchoiceUpdated calls. An artificial forkchoiceUpdated call will be created 4s before each slot, when the head updates and when a validator updates their information.

Additionally, I added strict ordering for calls from the `BeaconChain` to the `ExecutionLayer`. I'm not certain the `ExecutionLayer` will always maintain this ordering, but it's a good start to have consistency from the `BeaconChain`. There are some deadlock opportunities introduced, they are documented in the code.

## Additional Info

- ~~Blocked on #2837~~

Co-authored-by: realbigsean <[email protected]>
@bors bors bot changed the title Prepare proposer [Merged by Bors] - Prepare proposer Mar 9, 2022
@bors bors bot closed this Mar 9, 2022
bors bot pushed a commit that referenced this pull request Mar 10, 2022
## Issue Addressed

This address an issue which was preventing checkpoint-sync.

When the node starts from checkpoint sync, the head block and the finalized block are the same value. We did not respect this when sending a `forkchoiceUpdated` (fcU) call to the EL and were expecting fork choice to hold the *finalized ancestor of the head* and returning an error when it didn't.

This PR uses *only fork choice* for sending fcU updates. This is actually quite nice and avoids some atomicity issues between `chain.canonical_head` and `chain.fork_choice`. Now, whenever `chain.fork_choice.get_head` returns a value we also cache the values required for the next fcU call.

## TODO

- [x] ~~Blocked on #3043~~
- [x] Ensure there isn't a warn message at startup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bellatrix Required to support the Bellatrix Upgrade ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants